-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Post Type label for Document tab in Settings Header #17311
Use Post Type label for Document tab in Settings Header #17311
Conversation
packages/edit-post/src/components/sidebar/settings-header/index.js
Outdated
Show resolved
Hide resolved
packages/edit-post/src/components/sidebar/settings-header/index.js
Outdated
Show resolved
Hide resolved
packages/edit-post/src/components/sidebar/settings-header/index.js
Outdated
Show resolved
Hide resolved
packages/edit-post/src/components/sidebar/settings-header/index.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening this pull request.
To be clear. I understand the need to have more ways to customize the user interface. At the same time, I would like to notice that we don't use JS filters for UI labels in any place in the block editor. There are a few labels which are filterable using PHP filters, like the Paragraph block placeholder which is passed to the editor through its settings:
const { bodyPlaceholder } = getSettings(); |
Related PHP filter:
https://github.com/WordPress/wordpress-develop/blob/00b03f2a6f2c21ebf9975c9c08976358681400ba/src/wp-admin/edit-form-blocks.php#L270
This should be discussed first, what's the best strategy moving forward with adding new filters for UI labels and what naming conventions should be used before we proceed with this particular use case.
I also thought this to be an editor level setting first, but I felt it's not affecting the whole editor and only limited to the sidebar section. Also, I could not find any single place where all the labels are listed as a source of truth. So ended up adding the filter in the component itself. But if this is useful in wider sense, then taking a generic approach and a single collection of all the labels at one place makes more sense. My next question would be, which labels are good candidates to be part of this collection? Not everyone would like to customise them. So should it be part of WordPress Core or the Gutenberg plugin itself? |
@gziolo where is the best place to kick off this discussion? In this PR?, the #core-editor channel, elsewhere? |
Reminds me a bit of #12517 by the way :-) |
Wow, that's wild. I would not have imagined that. 😃 |
Sure, it sounds like a great topic to discuss.
I was thinking about it the other day. It's definitely one of the options as long as we ensure it performs well enough so you don't have to apply the same filters all over again on every component's re-render. I was also thinking about the alternatives we could investigate. I came up with one idea so far. Introduce Well, we could also discuss some set of constraints which would make the initial implementation safer if we do something like #12517. Example:
|
Thoughts on this from last Weekly Editor Chat:
|
@youknowriad, if I'm to take your approach of introducing new |
If we were to add a |
I was thinking, we might want to place it inside |
Oh. A new package of its own?
This should be fine, I guess. But, what qualifies for a new package? What purpose it's going to serve? What else it's going to contain?
This seems fine. I would not have much of an opinion on this (Lack of knowledge). Would defer to others. |
I was actually thinking about the |
It should work as well. Having @adamsilverstein - any thoughts on this topic as you are expert in terms of WordPress hooks :) |
|
…x.js Co-authored-by: Pascal Birchler <[email protected]>
…x.js Co-authored-by: Pascal Birchler <[email protected]>
@desaiuditd It seems to me that the Reusable Block post type label just needs to be changed to "Reusable Block". |
I think that's a perfectly acceptable risk. Labels are, after all, "official" user-facing names of a CPT. They are arguably expected to be succinct. For instance, the generic label Case in point: the label "A very long Custom Post Type name" still fits, even if not elegantly. It becomes a problem at three lines.
IMO this is the kind of issue that we'd only look at if and when it proves to be an issue. |
Related: #20877 (comment) That particular exploration from @dubielzyk brings the Document tab into the menu popover. |
@swissspidy @ZebulanStanphill @mcsf I've added the shim to override the post type labels for reusable blocks. Respective trac ticket to update it in WordPress Core Trac#50755 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for all the work.
Description
This PR initially started as ability to filter the Document label in the sidebar. I needed to be able to rename that label in my project. So that I could differentiate between different post types and custom post types as well. It could also be a generic custom label e.g.,
Article
,Story
etc. for any specific newsroom.After many iterations, eventually we decided to use Post Type label for this. So that we can also help in #21850. For my use case, I will be able to filter the post type labels probably using an existing WP filter
register_post_type_args
or maybe an existing WP action right before the Block Editor preloads initial datablock_editor_preload_paths
.How has this been tested?
Tested this on my local WordPress instance by checking Template and Template Parts CPTs pages.
Screenshots
Result after adding some custom code for my use case:
Result on the Template and Template Parts CPTs registered for FSE.
Types of changes
Document
. If the post type does not have the label value, then it will fallback toDocument
. This is a behaviour change.Checklist: